Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

WSL2 is a normal VM, why should it skip that step?

I'm not sure why the original check was there, but this seems like a reasonable extension of it. I'm ok to r+ but @mati865 if you'd prefer to discuss a bit more that's fine with me too.

I'm still not convinced this is the proper solution. WSL2 has proper ext4 filesystem that has proper permissions:

$ stat Projects/rust/Cargo.toml
  File: Projects/rust/Cargo.toml
  Size: 3291            Blocks: 8          IO Block: 4096   regular file
Device: 810h/2064d      Inode: 111745      Links: 1
Access: (0644/-rw-r--r--)  Uid: ( 1000/ mateusz)   Gid: ( 1000/ mateusz)
Access: 2020-07-27 16:06:23.850000000 +0200
Modify: 2020-07-27 16:06:23.480000000 +0200
Change: 2020-07-27 16:06:23.480000000 +0200
 Birth: -
$ cat /proc/version
Linux version 4.19.104-microsoft-standard (oe-user@oe-host) (gcc version 8.2.0 (GCC)) #1 SMP Wed Feb 19 06:37:35 UTC 2020

You really shouldn't use Rust from /mnt/X because all IO operations will be performed over P9 protocol which is awfully slow.

IMO the proper solution here would be to check if filesystem supports proper permissions and otherwise skip the check but I doubt that's easy to do.

I see. Maybe others in the windows notification group may have some advice =)

@rustbot ping windows

Hey Windows Group! This bug has been identified as a good "Windows candidate".
In case it's useful, here are some instructions for tackling these sorts of
bugs. Maybe take a look?
Thanks! <3

cc @arlosi @danielframpton @gdr-at-ms @kennykerr @luqmana @lzybkr @retep998 @rylev @sivadeilra

Adding T-compiler label so this can be picked up by our automated prioritization tool and added to the following T-compiler weekly meeting agenda.

This was discussed during today's compiler meeting, check out the log here

and there was further followup discussion today, here

The conclusion from yesterday had been "maybe we should revise things so that this check runs solely on CI, because the existing logic (with or without the change proposed here) isn't really doing the right thing -- you want to know properties of the file system, not of the host OS)."

But then today we rethought things more. maybe we'll still disable the check outside of CI, or maybe we'll come up with a more robust form of the check. See the discussion linked above.

I'm leaving this PR open for now, because if we do leave this logic in, then we should probably apply this amendment to it. But it would be better to figure out a way to revise the logic or remove it.

☔ The latest upstream changes (presumably #78060) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@Mark-Simulacrum it looks like your PR supersedes this, so I'll just close it?

Yes, that seems right, thank you!